From eaced67e7a7c35a3fc8be24ca8dcaab45fbd9ffa Mon Sep 17 00:00:00 2001 From: Twice Date: Wed, 25 Oct 2023 17:54:36 +0900 Subject: [PATCH] Fix status mis-return in JSON.CLEAR and polish code style (#1853) --- src/commands/cmd_json.cc | 2 +- src/types/json.h | 66 +++++++++++++++----------------- src/types/redis_json.cc | 12 ++++-- src/types/redis_json.h | 2 +- tests/cppunit/types/json_test.cc | 2 +- 5 files changed, 42 insertions(+), 42 deletions(-) diff --git a/src/commands/cmd_json.cc b/src/commands/cmd_json.cc index d17b3b556be..7ce58addfb1 100644 --- a/src/commands/cmd_json.cc +++ b/src/commands/cmd_json.cc @@ -153,7 +153,7 @@ class CommandJsonClear : public Commander { Status Execute(Server *svr, Connection *conn, std::string *output) override { redis::Json json(svr->storage, conn->GetNamespace()); - int result = 0; + size_t result = 0; // If path not specified set it to $ std::string path = (args_.size() > 2) ? args_[2] : "$"; diff --git a/src/types/json.h b/src/types/json.h index a099c470f53..8d56a92ad39 100644 --- a/src/types/json.h +++ b/src/types/json.h @@ -134,72 +134,68 @@ struct JsonValue { return result_count; } - Status Type(std::string_view path, std::vector *types) const { - types->clear(); + StatusOr> Type(std::string_view path) const { + std::vector types; try { jsoncons::jsonpath::json_query(value, path, [&types](const std::string & /*path*/, const jsoncons::json &val) { switch (val.type()) { case jsoncons::json_type::null_value: - types->emplace_back("null"); + types.emplace_back("null"); break; case jsoncons::json_type::bool_value: - types->emplace_back("boolean"); + types.emplace_back("boolean"); break; case jsoncons::json_type::int64_value: case jsoncons::json_type::uint64_value: - types->emplace_back("integer"); + types.emplace_back("integer"); break; case jsoncons::json_type::double_value: - types->emplace_back("number"); + types.emplace_back("number"); break; case jsoncons::json_type::string_value: - types->emplace_back("string"); + types.emplace_back("string"); break; case jsoncons::json_type::array_value: - types->emplace_back("array"); + types.emplace_back("array"); break; case jsoncons::json_type::object_value: - types->emplace_back("object"); + types.emplace_back("object"); break; default: - types->emplace_back("unknown"); + types.emplace_back("unknown"); break; } }); } catch (const jsoncons::jsonpath::jsonpath_error &e) { return {Status::NotOK, e.what()}; } - return Status::OK(); + + return types; } - Status Clear(std::string_view path, int *result) { + StatusOr Clear(std::string_view path) { + size_t count = 0; 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_array) - val = jsoncons::json::array(); - else if (is_object) - val = jsoncons::json::object(); - else if (is_number) - val = 0; - else - return; - - cleared_count++; - }); - - *result = cleared_count; + jsoncons::jsonpath::json_replace(value, path, [&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_array) + val = jsoncons::json::array(); + else if (is_object) + val = jsoncons::json::object(); + else if (is_number) + val = 0; + else + return; + + count++; + }); } catch (const jsoncons::jsonpath::jsonpath_error &e) { return {Status::NotOK, e.what()}; } - return Status::OK(); + return count; } JsonValue(const JsonValue &) = default; diff --git a/src/types/redis_json.cc b/src/types/redis_json.cc index f7b4e6d22e0..45785a2a25a 100644 --- a/src/types/redis_json.cc +++ b/src/types/redis_json.cc @@ -159,11 +159,14 @@ rocksdb::Status Json::Type(const std::string &user_key, const std::string &path, auto s = read(ns_key, &metadata, &json_val); if (!s.ok()) return s; - auto res = json_val.Type(path, results); + auto res = json_val.Type(path); + if (!res) return rocksdb::Status::InvalidArgument(res.Msg()); + + *results = *res; return rocksdb::Status::OK(); } -rocksdb::Status Json::Clear(const std::string &user_key, const std::string &path, int *result) { +rocksdb::Status Json::Clear(const std::string &user_key, const std::string &path, size_t *result) { auto ns_key = AppendNamespacePrefix(user_key); LockGuard guard(storage_->GetLockManager(), ns_key); @@ -174,9 +177,10 @@ rocksdb::Status Json::Clear(const std::string &user_key, const std::string &path if (!s.ok()) return s; - auto res = json_val.Clear(path, result); - if (!res.OK()) return s; + auto res = json_val.Clear(path); + if (!res) return rocksdb::Status::InvalidArgument(res.Msg()); + *result = *res; if (*result == 0) { return rocksdb::Status::OK(); } diff --git a/src/types/redis_json.h b/src/types/redis_json.h index c8b5b9b3841..cac45796512 100644 --- a/src/types/redis_json.h +++ b/src/types/redis_json.h @@ -38,7 +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); + rocksdb::Status Clear(const std::string &user_key, const std::string &path, size_t *result); private: rocksdb::Status write(Slice ns_key, JsonMetadata *metadata, const JsonValue &json_val); diff --git a/tests/cppunit/types/json_test.cc b/tests/cppunit/types/json_test.cc index 79ff4fd5994..738504b9eb5 100644 --- a/tests/cppunit/types/json_test.cc +++ b/tests/cppunit/types/json_test.cc @@ -183,7 +183,7 @@ TEST_F(RedisJsonTest, ArrAppend) { } TEST_F(RedisJsonTest, Clear) { - int result = 0; + size_t result = 0; ASSERT_TRUE( json_