refactor(json_family): code clean up

Signed-off-by: Stepan Bagritsevich <[email protected]>

Author: BagritsevichStepan

src/server/detail/wrapped_json_path.h

@@ -14,7 +14,7 @@
 
 namespace dfly {
 
-class Nothing {};
+using Nothing = std::monostate;
 using JsonExpression = jsoncons::jsonpath::jsonpath_expression<JsonType>;
 
 template <typename T>
@@ -24,11 +24,11 @@ template <typename T = Nothing> class MutateCallbackResult {
  public:
   MutateCallbackResult() = default;
 
-  explicit MutateCallbackResult(bool should_be_deleted_) : should_be_deleted(should_be_deleted_) {
+  explicit MutateCallbackResult(bool should_be_deleted) : should_be_deleted_(should_be_deleted_) {
   }
 
-  MutateCallbackResult(bool should_be_deleted_, T&& value)
-      : should_be_deleted(should_be_deleted_), value_(std::forward<T>(value)) {
+  MutateCallbackResult(bool should_be_deleted, T&& value)
+      : should_be_deleted_(should_be_deleted), value_(std::forward<T>(value)) {
   }
 
   bool HasValue() const {
@@ -39,9 +39,12 @@ template <typename T = Nothing> class MutateCallbackResult {
     return std::move(value_).value();
   }
 
-  bool should_be_deleted;
+  bool ShouldBeDeleted() const {
+    return should_be_deleted_;
+  }
 
  private:
+  bool should_be_deleted_;
   std::optional<T> value_;
 };
 
@@ -81,6 +84,9 @@ template <typename T> class JsonCallbackResult {
     }
   }
 
+  explicit JsonCallbackResult(std::error_code error_code) : error_code_(error_code) {
+  }
+
   void AddValue(T&& value) {
     if (IsV1()) {
       details::OptionalEmplace(std::forward<T>(value), &AsV1());
@@ -94,27 +100,37 @@ template <typename T> class JsonCallbackResult {
   }
 
   JsonV1Result& AsV1() {
+    DCHECK(!ErrorOccured());
     return std::get<JsonV1Result>(result_);
   }
 
   JsonV2Result& AsV2() {
+    DCHECK(!ErrorOccured());
     return std::get<JsonV2Result>(result_);
   }
 
   const JsonV1Result& AsV1() const {
+    DCHECK(!ErrorOccured());
     return std::get<JsonV1Result>(result_);
   }
 
   const JsonV2Result& AsV2() const {
+    DCHECK(!ErrorOccured());
     return std::get<JsonV2Result>(result_);
   }
 
- public:
-  std::error_code error_code;
+  bool ErrorOccured() const {
+    return static_cast<bool>(error_code_);
+  }
+
+  const std::error_code& GetError() const {
+    return error_code_;
+  }
 
  private:
   std::variant<JsonV1Result, JsonV2Result> result_;
   bool legacy_mode_is_enabled_;
+  std::error_code error_code_;
 };
 
 class WrappedJsonPath {
@@ -173,40 +189,39 @@ class WrappedJsonPath {
       if (res.HasValue()) {
         mutate_result.AddValue(std::move(res).GetValue());
       }
-      return res.should_be_deleted;
+      return res.ShouldBeDeleted();
     };
 
     if (HoldsJsonPath()) {
       const auto& json_path = AsJsonPath();
       json::MutatePath(json_path, mutate_callback, json_entry);
     } else {
-      using evaluator_t = jsoncons::jsonpath::detail::jsonpath_evaluator<JsonType, JsonType&>;
-      using value_type = evaluator_t::value_type;
-      using reference = evaluator_t::reference;
-      using json_selector_t = evaluator_t::path_expression_type;
+      using namespace jsoncons::jsonpath;
+      using namespace jsoncons::jsonpath::detail;
+      using Evaluator = jsonpath_evaluator<JsonType, JsonType&>;
+      using ValueType = Evaluator::value_type;
+      using Reference = Evaluator::reference;
+      using JsonSelector = Evaluator::path_expression_type;
 
-      jsoncons::jsonpath::custom_functions<JsonType> funcs =
-          jsoncons::jsonpath::custom_functions<JsonType>();
+      custom_functions<JsonType> funcs = custom_functions<JsonType>();
 
-      auto& ec = mutate_result.error_code;
-      jsoncons::jsonpath::detail::static_resources<value_type, reference> static_resources(funcs);
-      evaluator_t e;
+      std::error_code ec;
+      static_resources<ValueType, Reference> static_resources(funcs);
+      Evaluator e;
 
-      json_selector_t expr = e.compile(static_resources, path_.view(), ec);
+      JsonSelector expr = e.compile(static_resources, path_.view(), ec);
       if (ec) {
-        return mutate_result;
+        return JsonCallbackResult<T>{ec};
       }
 
-      jsoncons::jsonpath::detail::dynamic_resources<value_type, reference> resources;
+      dynamic_resources<ValueType, Reference> resources;
 
-      auto f = [&mutate_callback](const jsoncons::jsonpath::basic_path_node<char>& path,
-                                  JsonType& val) {
-        mutate_callback(jsoncons::jsonpath::to_string(path), &val);
+      auto f = [&mutate_callback](const basic_path_node<char>& path, JsonType& val) {
+        mutate_callback(to_string(path), &val);
       };
 
-      expr.evaluate(
-          resources, *json_entry, json_selector_t::path_node_type{}, *json_entry, std::move(f),
-          jsoncons::jsonpath::result_options::nodups | jsoncons::jsonpath::result_options::path);
+      expr.evaluate(resources, *json_entry, JsonSelector::path_node_type{}, *json_entry,
+                    std::move(f), result_options::nodups | result_options::path);
     }
     return mutate_result;
   }

src/server/json_family.cc

@@ -134,8 +134,8 @@ template <typename T> void Send(const std::vector<T>& vec, RedisReplyBuilder* rb
 }
 
 template <typename T> void Send(const JsonCallbackResult<T>& result, RedisReplyBuilder* rb) {
-  if (result.error_code) {
-    rb->SendError(result.error_code.message());
+  if (result.ErrorOccured()) {
+    rb->SendError(result.GetError().message());
     return;
   }
 
@@ -295,7 +295,7 @@ error_code JsonReplace(JsonType& instance, string_view path, json::MutateCallbac
   return ec;
 }
 
-template <typename T>
+template <typename T = Nothing>
 OpResult<JsonCallbackResult<T>> UpdateEntry(const OpArgs& op_args, std::string_view key,
                                             const WrappedJsonPath& json_path,
                                             JsonPathMutateCallback<T> cb,
@@ -308,18 +308,17 @@ OpResult<JsonCallbackResult<T>> UpdateEntry(const OpArgs& op_args, std::string_v
   JsonType* json_val = pv.GetJson();
   DCHECK(json_val) << "should have a valid JSON object for key '" << key << "' the type for it is '"
                    << pv.ObjType() << "'";
-  JsonType& json_entry = *json_val;
 
   op_args.shard->search_indices()->RemoveDoc(key, op_args.db_cntx, pv);
 
   auto mutate_res = json_path.Mutate(json_val, cb);
-  if (mutate_res.error_code) {
-    return mutate_res;  // TODO(continue)
+  if (mutate_res.ErrorOccured()) {
+    return mutate_res;  // TODO(If error occured return or continue)
   }
 
   // Make sure that we don't have other internal issue with the operation
   if (verify_op) {
-    verify_op(json_entry);
+    verify_op(*json_val);
   }
 
   it_res->post_updater.Run();