From b8d26b21ead69e89f7a7863fb29bcf4c2bb6613d Mon Sep 17 00:00:00 2001 From: Dima Kachurovskyy Date: Sun, 22 Oct 2023 06:58:06 -0400 Subject: [PATCH 1/8] [WIP] JSON Clean command --- kvrocks.conf | 13 ++++++++++++- src/commands/cmd_json.cc | 17 +++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/kvrocks.conf b/kvrocks.conf index be4332543d8..7ca806e3ba1 100644 --- a/kvrocks.conf +++ b/kvrocks.conf @@ -135,7 +135,7 @@ log-retention-days -1 # When running in daemonize mode, kvrocks writes a PID file in ${CONFIG_DIR}/kvrocks.pid by # default. You can specify a custom pid file location here. # pidfile /var/run/kvrocks.pid -pidfile "" +pidfile /tmp/kvrocks/kvrocks.pid # You can configure a slave instance to accept writes or not. Writing against # a slave instance may be useful to store some ephemeral data (because data @@ -859,3 +859,14 @@ rocksdb.rate_limiter_auto_tuned yes ################################ NAMESPACE ##################################### # namespace.test change.me +auto-resize-block-and-sst yes +backup-dir /tmp/kvrocks/backup +force-compact-file-age 172800 +force-compact-file-min-deleted-percentage 10 +fullsync-recv-file-delay 0 +persist-cluster-nodes-enabled yes +replica-announce-port 0 +rocksdb.metadata_block_cache_size 2048 +rocksdb.share_metadata_and_subkey_block_cache yes +rocksdb.subkey_block_cache_size 2048 +unixsocketperm 777 diff --git a/src/commands/cmd_json.cc b/src/commands/cmd_json.cc index 2c0fb0c68e3..6517f8addfa 100644 --- a/src/commands/cmd_json.cc +++ b/src/commands/cmd_json.cc @@ -19,11 +19,14 @@ */ #include +#include #include "commander.h" #include "commands/command_parser.h" +#include "server/redis_connection.h" #include "server/redis_reply.h" #include "server/server.h" +#include "status.h" #include "types/redis_json.h" namespace redis { @@ -100,6 +103,20 @@ class CommandJsonGet : public Commander { std::vector paths_; }; + +class CommandJsonClear : public Commander { +public: + Status Execute(Server *svr, Connection *conn, std::string *output) override { + std::cout << "Server parameter: " << svr << std::endl; + std::cout << "Connection parameter: " << conn << std::endl; + std::cout << "Output parameter: " << *output << std::endl; + + redis::Json json(svr->storage, conn->GetNamespace()); + + return Status::OK(); + } +}; + class CommandJsonArrAppend : public Commander { public: Status Execute(Server *svr, Connection *conn, std::string *output) override { From f1757869e9c8f8e87c3ee5c1958f6f9f6adbe34d Mon Sep 17 00:00:00 2001 From: Dima Kachurovskyy Date: Sun, 22 Oct 2023 12:15:35 -0400 Subject: [PATCH 2/8] Added command for handling JSON.CLEAR command --- src/commands/cmd_json.cc | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/src/commands/cmd_json.cc b/src/commands/cmd_json.cc index 6517f8addfa..f9c7511d6df 100644 --- a/src/commands/cmd_json.cc +++ b/src/commands/cmd_json.cc @@ -103,20 +103,6 @@ class CommandJsonGet : public Commander { std::vector paths_; }; - -class CommandJsonClear : public Commander { -public: - Status Execute(Server *svr, Connection *conn, std::string *output) override { - std::cout << "Server parameter: " << svr << std::endl; - std::cout << "Connection parameter: " << conn << std::endl; - std::cout << "Output parameter: " << *output << std::endl; - - redis::Json json(svr->storage, conn->GetNamespace()); - - return Status::OK(); - } -}; - class CommandJsonArrAppend : public Commander { public: Status Execute(Server *svr, Connection *conn, std::string *output) override { @@ -165,9 +151,33 @@ class CommandJsonType : public Commander { } }; +class CommandJsonClear : public Commander { + public: + Status Execute(Server *svr, Connection *conn, std::string *output) override { + redis::Json json(svr->storage, conn->GetNamespace()); + + int result = 0; + + // If path not specified set it to $ + std::string path = (args_.size() > 2) ? args_[2] : "$"; + auto s = json.Clear(args_[1], path, &result); + + if (s.IsNotFound()) { + *output = redis::NilString(); + return Status::OK(); + } + + if (!s.ok()) return {Status::RedisExecErr, s.ToString()}; + + *output = redis::Integer(result); + return Status::OK(); + } +}; + REDIS_REGISTER_COMMANDS(MakeCmdAttr("json.set", -3, "write", 1, 1, 1), MakeCmdAttr("json.get", -2, "read-only", 1, 1, 1), MakeCmdAttr("json.type", -2, "read-only", 1, 1, 1), - MakeCmdAttr("json.arrappend", -4, "write", 1, 1, 1), ); + MakeCmdAttr("json.arrappend", -4, "write", 1, 1, 1), + MakeCmdAttr("json.clear", -2, "write", 1, 1, 1), ); } // namespace redis From d4eea16a5fbd516c5ce9512a467272f05cebe8db Mon Sep 17 00:00:00 2001 From: Dima Kachurovskyy Date: Sun, 22 Oct 2023 12:16:32 -0400 Subject: [PATCH 3/8] Added method initialization for JSON.CLEAR command in .h file --- src/types/redis_json.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/types/redis_json.h b/src/types/redis_json.h index d9492706dde..c8b5b9b3841 100644 --- a/src/types/redis_json.h +++ b/src/types/redis_json.h @@ -38,6 +38,7 @@ class Json : public Database { rocksdb::Status Type(const std::string &user_key, const std::string &path, std::vector *results); rocksdb::Status ArrAppend(const std::string &user_key, const std::string &path, const std::vector &values, std::vector *result_count); + rocksdb::Status Clear(const std::string &user_key, const std::string &path, int *result); private: rocksdb::Status write(Slice ns_key, JsonMetadata *metadata, const JsonValue &json_val); From 313518851dcc7f81f3007928ea9f577bf3741f88 Mon Sep 17 00:00:00 2001 From: Dima Kachurovskyy Date: Sun, 22 Oct 2023 12:17:45 -0400 Subject: [PATCH 4/8] Added implementation of JSON.CLEAR command using jsoncons_ext --- kvrocks.conf | 13 +------------ src/commands/cmd_json.cc | 3 --- src/types/json.h | 27 +++++++++++++++++++++++++++ src/types/redis_json.cc | 20 ++++++++++++++++++++ 4 files changed, 48 insertions(+), 15 deletions(-) diff --git a/kvrocks.conf b/kvrocks.conf index 7ca806e3ba1..be4332543d8 100644 --- a/kvrocks.conf +++ b/kvrocks.conf @@ -135,7 +135,7 @@ log-retention-days -1 # When running in daemonize mode, kvrocks writes a PID file in ${CONFIG_DIR}/kvrocks.pid by # default. You can specify a custom pid file location here. # pidfile /var/run/kvrocks.pid -pidfile /tmp/kvrocks/kvrocks.pid +pidfile "" # You can configure a slave instance to accept writes or not. Writing against # a slave instance may be useful to store some ephemeral data (because data @@ -859,14 +859,3 @@ rocksdb.rate_limiter_auto_tuned yes ################################ NAMESPACE ##################################### # namespace.test change.me -auto-resize-block-and-sst yes -backup-dir /tmp/kvrocks/backup -force-compact-file-age 172800 -force-compact-file-min-deleted-percentage 10 -fullsync-recv-file-delay 0 -persist-cluster-nodes-enabled yes -replica-announce-port 0 -rocksdb.metadata_block_cache_size 2048 -rocksdb.share_metadata_and_subkey_block_cache yes -rocksdb.subkey_block_cache_size 2048 -unixsocketperm 777 diff --git a/src/commands/cmd_json.cc b/src/commands/cmd_json.cc index f9c7511d6df..1904255984c 100644 --- a/src/commands/cmd_json.cc +++ b/src/commands/cmd_json.cc @@ -19,14 +19,11 @@ */ #include -#include #include "commander.h" #include "commands/command_parser.h" -#include "server/redis_connection.h" #include "server/redis_reply.h" #include "server/server.h" -#include "status.h" #include "types/redis_json.h" namespace redis { diff --git a/src/types/json.h b/src/types/json.h index a3dd4c9521e..46cea12140b 100644 --- a/src/types/json.h +++ b/src/types/json.h @@ -172,6 +172,33 @@ struct JsonValue { return Status::OK(); } + Status Clear(std::string_view path, int *result) { + try { + int cleared_count = 0; + jsoncons::jsonpath::json_replace( + value, path, [&cleared_count](const std::string & + /*path*/, jsoncons::json &val) { + bool is_array = val.is_array() && !val.empty(); + bool is_object = val.is_object() && !val.empty(); + bool is_number = val.is_number() && val.as() != 0; + + if (!is_number && !is_array && !is_object) { + return; + } + + if (is_array) val = jsoncons::json::array(); + if (is_object) val = jsoncons::json::object(); + if (is_number) val = 0; + cleared_count++; + }); + + *result = cleared_count; + } catch (const jsoncons::jsonpath::jsonpath_error &e) { + return {Status::NotOK, e.what()}; + } + return Status::OK(); + } + JsonValue(const JsonValue &) = default; JsonValue(JsonValue &&) = default; diff --git a/src/types/redis_json.cc b/src/types/redis_json.cc index 7381fef64b0..f7b4e6d22e0 100644 --- a/src/types/redis_json.cc +++ b/src/types/redis_json.cc @@ -163,4 +163,24 @@ rocksdb::Status Json::Type(const std::string &user_key, const std::string &path, return rocksdb::Status::OK(); } +rocksdb::Status Json::Clear(const std::string &user_key, const std::string &path, int *result) { + auto ns_key = AppendNamespacePrefix(user_key); + + LockGuard guard(storage_->GetLockManager(), ns_key); + + JsonValue json_val; + JsonMetadata metadata; + auto s = read(ns_key, &metadata, &json_val); + + if (!s.ok()) return s; + + auto res = json_val.Clear(path, result); + if (!res.OK()) return s; + + if (*result == 0) { + return rocksdb::Status::OK(); + } + + return write(ns_key, &metadata, json_val); +} } // namespace redis From 16600e3f7da3ac8fa7be367895c50eb87697fc41 Mon Sep 17 00:00:00 2001 From: Dima Kachurovskyy Date: Sun, 22 Oct 2023 13:00:21 -0400 Subject: [PATCH 5/8] Added CPP Tests --- tests/cppunit/types/json_test.cc | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/cppunit/types/json_test.cc b/tests/cppunit/types/json_test.cc index e3be9774f16..bb760d7cbff 100644 --- a/tests/cppunit/types/json_test.cc +++ b/tests/cppunit/types/json_test.cc @@ -181,3 +181,35 @@ TEST_F(RedisJsonTest, ArrAppend) { R"({"x":[1,2,{"x":[1,2,{"y":[1,2,3],"z":3}],"y":[{"y":1},1,2,3]},1],"y":[1,2,3,1,2,3]})"); res.clear(); } + +TEST_F(RedisJsonTest, Clear) { + int result = 0; + + ASSERT_TRUE(json_->Set(key_, "$", R"({"obj":{"a":1, "b":2}, "arr":[1,2,3], "str": "foo", "bool": true, "int": 42, "float": 3.14})").ok()); + + ASSERT_TRUE(json_->Clear(key_, "$", &result).ok()); + ASSERT_TRUE(json_->Get(key_, {}, &json_val_).ok()); + ASSERT_EQ(json_val_.Dump().GetValue(), "{}"); + ASSERT_EQ(result, 1); + + ASSERT_TRUE(json_->Set(key_, "$", R"({"obj":{"a":1, "b":2}, "arr":[1,2,3], "str": "foo", "bool": true, "int": 42, "float": 3.14})").ok()); + + ASSERT_TRUE(json_->Clear(key_, "$.obj", &result).ok()); + ASSERT_TRUE(json_->Get(key_, {}, &json_val_).ok()); + ASSERT_EQ(json_val_.Dump().GetValue(), R"({"arr":[1,2,3],"bool":true,"float":3.14,"int":42,"obj":{},"str":"foo"})"); + ASSERT_EQ(result, 1); + + ASSERT_TRUE(json_->Clear(key_, "$.arr", &result).ok()); + ASSERT_TRUE(json_->Get(key_, {}, &json_val_).ok()); + ASSERT_EQ(json_val_.Dump().GetValue(), R"({"arr":[],"bool":true,"float":3.14,"int":42,"obj":{},"str":"foo"})"); + ASSERT_EQ(result, 1); + + ASSERT_TRUE(json_->Set(key_, "$", R"({"obj":{"a":1, "b":2}, "arr":[1,2,3], "str": "foo", "bool": true, "int": 42, "float": 3.14})").ok()); + ASSERT_TRUE(json_->Clear(key_, "$.*", &result).ok()); + ASSERT_TRUE(json_->Get(key_, {}, &json_val_).ok()); + ASSERT_EQ(json_val_.Dump().GetValue(), R"({"arr":[],"bool":true,"float":0,"int":0,"obj":{},"str":"foo"})"); + ASSERT_EQ(result, 4); + + ASSERT_TRUE(json_->Clear(key_, "$.some", &result).ok()); + ASSERT_EQ(result, 0); +} From 7025f3f08a4f8f3fb9129c31ae2217b2d26fa65d Mon Sep 17 00:00:00 2001 From: Dima Kachurovskyy Date: Sun, 22 Oct 2023 13:05:42 -0400 Subject: [PATCH 6/8] Added go tests --- src/commands/cmd_json.cc | 3 ++- tests/gocase/unit/type/json/json_test.go | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/commands/cmd_json.cc b/src/commands/cmd_json.cc index 1904255984c..247535a7eb6 100644 --- a/src/commands/cmd_json.cc +++ b/src/commands/cmd_json.cc @@ -171,7 +171,8 @@ class CommandJsonClear : public Commander { } }; -REDIS_REGISTER_COMMANDS(MakeCmdAttr("json.set", -3, "write", 1, 1, 1), + +REDIS_REGISTER_COMMANDS(MakeCmdAttr("json.set", 4, "write", 1, 1, 1), MakeCmdAttr("json.get", -2, "read-only", 1, 1, 1), MakeCmdAttr("json.type", -2, "read-only", 1, 1, 1), MakeCmdAttr("json.arrappend", -4, "write", 1, 1, 1), diff --git a/tests/gocase/unit/type/json/json_test.go b/tests/gocase/unit/type/json/json_test.go index b86e343fc4f..2a0cd01e3b2 100644 --- a/tests/gocase/unit/type/json/json_test.go +++ b/tests/gocase/unit/type/json/json_test.go @@ -37,6 +37,7 @@ func TestJson(t *testing.T) { defer func() { require.NoError(t, rdb.Close()) }() t.Run("JSON.SET and JSON.GET basics", func(t *testing.T) { + require.Error(t, rdb.Do(ctx, "JSON.SET", "a").Err()) require.NoError(t, rdb.Do(ctx, "JSON.SET", "a", "$", ` {"x":1, "y":2} `).Err()) require.Equal(t, rdb.Do(ctx, "JSON.GET", "a").Val(), `{"x":1,"y":2}`) @@ -129,4 +130,26 @@ func TestJson(t *testing.T) { _, err = rdb.Do(ctx, "JSON.TYPE", "not_exists", "$").StringSlice() require.EqualError(t, err, redis.Nil.Error()) }) + + t.Run("Clear JSON values", func(t *testing.T) { + require.NoError(t, rdb.Do(ctx, "JSON.SET", "bb", "$", `{"obj":{"a":1, "b":2}, "arr":[1,2,3], "str": "foo", "bool": true, "int": 42, "float": 3.14}`).Err()) + + require.NoError(t, rdb.Do(ctx, "JSON.CLEAR", "bb", "$").Err()) + require.Equal(t, `{}`, rdb.Do(ctx, "JSON.GET", "bb").Val()) + + require.NoError(t, rdb.Do(ctx, "JSON.SET", "bb", "$", `{"obj":{"a":1, "b":2}, "arr":[1,2,3], "str": "foo", "bool": true, "int": 42, "float": 3.14}`).Err()) + require.NoError(t, rdb.Do(ctx, "JSON.CLEAR", "bb", "$.obj").Err()) + require.Equal(t, `{"arr":[1,2,3],"bool":true,"float":3.14,"int":42,"obj":{},"str":"foo"}`, rdb.Do(ctx, "JSON.GET", "bb").Val()) + + require.NoError(t, rdb.Do(ctx, "JSON.CLEAR", "bb", "$.arr").Err()) + require.Equal(t, `{"arr":[],"bool":true,"float":3.14,"int":42,"obj":{},"str":"foo"}`, rdb.Do(ctx, "JSON.GET", "bb").Val()) + + require.NoError(t, rdb.Do(ctx, "JSON.SET", "bb", "$", `{"obj":{"a":1, "b":2}, "arr":[1,2,3], "str": "foo", "bool": true, "int": 42, "float": 3.14}`).Err()) + require.NoError(t, rdb.Do(ctx, "JSON.CLEAR", "bb", "$.*").Err()) + require.Equal(t, `{"arr":[],"bool":true,"float":0,"int":0,"obj":{},"str":"foo"}`, rdb.Do(ctx, "JSON.GET", "bb").Val()) + + _, err := rdb.Do(ctx, "JSON.CLEAR", "bb", "$.some").Result() + require.NoError(t, err) + }) + } From 5b4a7b37a77544c617fee7fe775bb380966618b3 Mon Sep 17 00:00:00 2001 From: Dima Kachurovskyy Date: Sun, 22 Oct 2023 17:28:37 -0400 Subject: [PATCH 7/8] runned formatting, go lint and tidy. --- src/commands/cmd_json.cc | 1 - src/types/json.h | 33 ++++++++++++++++---------------- tests/cppunit/types/json_test.cc | 18 ++++++++++++++--- 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/commands/cmd_json.cc b/src/commands/cmd_json.cc index 247535a7eb6..d17b3b556be 100644 --- a/src/commands/cmd_json.cc +++ b/src/commands/cmd_json.cc @@ -171,7 +171,6 @@ class CommandJsonClear : public Commander { } }; - REDIS_REGISTER_COMMANDS(MakeCmdAttr("json.set", 4, "write", 1, 1, 1), MakeCmdAttr("json.get", -2, "read-only", 1, 1, 1), MakeCmdAttr("json.type", -2, "read-only", 1, 1, 1), diff --git a/src/types/json.h b/src/types/json.h index 46cea12140b..fbcb956f947 100644 --- a/src/types/json.h +++ b/src/types/json.h @@ -175,22 +175,23 @@ struct JsonValue { Status Clear(std::string_view path, int *result) { try { int cleared_count = 0; - jsoncons::jsonpath::json_replace( - value, path, [&cleared_count](const std::string & - /*path*/, jsoncons::json &val) { - bool is_array = val.is_array() && !val.empty(); - bool is_object = val.is_object() && !val.empty(); - bool is_number = val.is_number() && val.as() != 0; - - if (!is_number && !is_array && !is_object) { - return; - } - - if (is_array) val = jsoncons::json::array(); - if (is_object) val = jsoncons::json::object(); - if (is_number) val = 0; - cleared_count++; - }); + jsoncons::jsonpath::json_replace(value, path, + [&cleared_count](const std::string & + /*path*/, + jsoncons::json &val) { + bool is_array = val.is_array() && !val.empty(); + bool is_object = val.is_object() && !val.empty(); + bool is_number = val.is_number() && val.as() != 0; + + if (!is_number && !is_array && !is_object) { + return; + } + + if (is_array) val = jsoncons::json::array(); + if (is_object) val = jsoncons::json::object(); + if (is_number) val = 0; + cleared_count++; + }); *result = cleared_count; } catch (const jsoncons::jsonpath::jsonpath_error &e) { diff --git a/tests/cppunit/types/json_test.cc b/tests/cppunit/types/json_test.cc index bb760d7cbff..79ff4fd5994 100644 --- a/tests/cppunit/types/json_test.cc +++ b/tests/cppunit/types/json_test.cc @@ -185,14 +185,22 @@ TEST_F(RedisJsonTest, ArrAppend) { TEST_F(RedisJsonTest, Clear) { int result = 0; - ASSERT_TRUE(json_->Set(key_, "$", R"({"obj":{"a":1, "b":2}, "arr":[1,2,3], "str": "foo", "bool": true, "int": 42, "float": 3.14})").ok()); + ASSERT_TRUE( + json_ + ->Set(key_, "$", + R"({"obj":{"a":1, "b":2}, "arr":[1,2,3], "str": "foo", "bool": true, "int": 42, "float": 3.14})") + .ok()); ASSERT_TRUE(json_->Clear(key_, "$", &result).ok()); ASSERT_TRUE(json_->Get(key_, {}, &json_val_).ok()); ASSERT_EQ(json_val_.Dump().GetValue(), "{}"); ASSERT_EQ(result, 1); - ASSERT_TRUE(json_->Set(key_, "$", R"({"obj":{"a":1, "b":2}, "arr":[1,2,3], "str": "foo", "bool": true, "int": 42, "float": 3.14})").ok()); + ASSERT_TRUE( + json_ + ->Set(key_, "$", + R"({"obj":{"a":1, "b":2}, "arr":[1,2,3], "str": "foo", "bool": true, "int": 42, "float": 3.14})") + .ok()); ASSERT_TRUE(json_->Clear(key_, "$.obj", &result).ok()); ASSERT_TRUE(json_->Get(key_, {}, &json_val_).ok()); @@ -204,7 +212,11 @@ TEST_F(RedisJsonTest, Clear) { ASSERT_EQ(json_val_.Dump().GetValue(), R"({"arr":[],"bool":true,"float":3.14,"int":42,"obj":{},"str":"foo"})"); ASSERT_EQ(result, 1); - ASSERT_TRUE(json_->Set(key_, "$", R"({"obj":{"a":1, "b":2}, "arr":[1,2,3], "str": "foo", "bool": true, "int": 42, "float": 3.14})").ok()); + ASSERT_TRUE( + json_ + ->Set(key_, "$", + R"({"obj":{"a":1, "b":2}, "arr":[1,2,3], "str": "foo", "bool": true, "int": 42, "float": 3.14})") + .ok()); ASSERT_TRUE(json_->Clear(key_, "$.*", &result).ok()); ASSERT_TRUE(json_->Get(key_, {}, &json_val_).ok()); ASSERT_EQ(json_val_.Dump().GetValue(), R"({"arr":[],"bool":true,"float":0,"int":0,"obj":{},"str":"foo"})"); From 111c5a9adffda81d65ce509857972890fba5749c Mon Sep 17 00:00:00 2001 From: Dima Kachurovskyy Date: Mon, 23 Oct 2023 12:53:20 -0400 Subject: [PATCH 8/8] make suggested changes related to conditions --- src/types/json.h | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/types/json.h b/src/types/json.h index fbcb956f947..a099c470f53 100644 --- a/src/types/json.h +++ b/src/types/json.h @@ -183,13 +183,15 @@ struct JsonValue { bool is_object = val.is_object() && !val.empty(); bool is_number = val.is_number() && val.as() != 0; - if (!is_number && !is_array && !is_object) { + if (is_array) + val = jsoncons::json::array(); + else if (is_object) + val = jsoncons::json::object(); + else if (is_number) + val = 0; + else return; - } - if (is_array) val = jsoncons::json::array(); - if (is_object) val = jsoncons::json::object(); - if (is_number) val = 0; cleared_count++; });