diff --git a/src/core/json/json_object.cc b/src/core/json/json_object.cc index 4d30e8888896..c9ffdcc1b85b 100644 --- a/src/core/json/json_object.cc +++ b/src/core/json/json_object.cc @@ -24,7 +24,7 @@ optional JsonFromString(string_view input, PMR_NS::memory_resource* mr parser.update(input); parser.finish_parse(decoder, ec); - if (decoder.is_valid()) { + if (!ec && decoder.is_valid()) { return decoder.get_result(); } return nullopt; diff --git a/src/server/json_family.cc b/src/server/json_family.cc index 19172d503972..45e91396cd45 100644 --- a/src/server/json_family.cc +++ b/src/server/json_family.cc @@ -319,7 +319,9 @@ bool JsonAreEquals(const JsonType& lhs, const JsonType& rhs) { } } -OpResult SetJson(const OpArgs& op_args, string_view key, JsonType value) { +template +OpResult SetJson(const OpArgs& op_args, string_view key, + Callback parse_json_value) { auto& db_slice = op_args.GetDbSlice(); auto op_res = db_slice.AddOrFind(op_args.db_cntx, key); @@ -329,14 +331,17 @@ OpResult SetJson(const OpArgs& op_args, string_view ke op_args.shard->search_indices()->RemoveDoc(key, op_args.db_cntx, res.it->second); + OpResult json_value = parse_json_value(); + RETURN_ON_BAD_STATUS(json_value); + if (JsonEnconding() == kEncodingJsonFlat) { flexbuffers::Builder fbb; - json::FromJsonType(value, &fbb); + json::FromJsonType(json_value.value(), &fbb); fbb.Finish(); const auto& buf = fbb.GetBuffer(); res.it->second.SetJson(buf.data(), buf.size()); } else { - res.it->second.SetJson(std::move(value)); + res.it->second.SetJson(*std::move(json_value)); } op_args.shard->search_indices()->AddDoc(key, op_args.db_cntx, res.it->second); return std::move(res); @@ -1280,11 +1285,14 @@ auto OpResp(const OpArgs& op_args, string_view key, const WrappedJsonPath& json_ OpResult OpSet(const OpArgs& op_args, string_view key, string_view path, const WrappedJsonPath& json_path, std::string_view json_str, bool is_nx_condition, bool is_xx_condition) { - std::optional parsed_json = ShardJsonFromString(json_str); - if (!parsed_json) { - VLOG(1) << "got invalid JSON string '" << json_str << "' cannot be saved"; - return OpStatus::SYNTAX_ERR; - } + auto parse_json = [&json_str]() -> OpResult { + std::optional parsed_json = ShardJsonFromString(json_str); + if (!parsed_json) { + VLOG(1) << "got invalid JSON string '" << json_str << "' cannot be saved"; + return OpStatus::SYNTAX_ERR; + } + return parsed_json.value(); + }; // The whole key should be replaced. // NOTE: unlike in Redis, we are overriding the value when the path is "$" @@ -1310,10 +1318,8 @@ OpResult OpSet(const OpArgs& op_args, string_view key, string_view path, // of this function. Because of this, the memory tracking will be off. Another solution here, // is to use absl::Cleanup and dispatch another Find() but that's too complicated because then // you need to take into account the order of destructors. - OpResult st = SetJson(op_args, key, parsed_json.value()); - if (st.status() != OpStatus::OK) { - return st.status(); - } + OpResult st = SetJson(op_args, key, std::move(parse_json)); + RETURN_ON_BAD_STATUS(st); mem_tracker.SetJsonSize(st->it->second, st->is_new); return true; } @@ -1325,7 +1331,11 @@ OpResult OpSet(const OpArgs& op_args, string_view key, string_view path, // then the assign here is called N times, where N == array.size(). bool path_exists = false; bool operation_result = false; + + OpResult parsed_json = parse_json(); + RETURN_ON_BAD_STATUS(parsed_json); const JsonType& new_json = parsed_json.value(); + auto cb = [&](std::optional, JsonType* val) -> MutateCallbackResult<> { path_exists = true; if (!is_nx_condition) {