Skip to content

Commit 2056fbe

Browse files
refactor(json_family): address comments
Signed-off-by: Stepan Bagritsevich <[email protected]>
1 parent 6934c95 commit 2056fbe

File tree

2 files changed

+39
-27
lines changed

2 files changed

+39
-27
lines changed

src/server/detail/wrapped_json_path.h

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44

55
#pragma once
66

7-
#include <absl/container/inlined_vector.h>
8-
97
#include <string_view>
108
#include <utility>
119
#include <variant>
@@ -16,7 +14,7 @@
1614

1715
namespace dfly {
1816

19-
using Nothing = boost::blank;
17+
class Nothing {};
2018
using JsonExpression = jsoncons::jsonpath::jsonpath_expression<JsonType>;
2119

2220
template <typename T>
@@ -53,22 +51,25 @@ using JsonPathMutateCallback =
5351

5452
namespace details {
5553

56-
template <typename T> void OptionalEmplace(std::optional<T>& optional, T&& value) {
57-
optional.emplace(std::forward<T>(value));
54+
template <typename T> void OptionalEmplace(T&& value, std::optional<T>* optional) {
55+
optional->emplace(std::forward<T>(value));
5856
}
5957

6058
template <typename T>
61-
void OptionalEmplace(std::optional<std::optional<T>>& optional, std::optional<T>&& value) {
59+
void OptionalEmplace(std::optional<T>&& value, std::optional<std::optional<T>>* optional) {
6260
if (value.has_value()) {
63-
optional.emplace(std::forward<std::optional<T>>(value));
61+
optional->emplace(std::forward<std::optional<T>>(value));
6462
}
6563
}
6664

6765
} // namespace details
6866

6967
template <typename T> class JsonCallbackResult {
7068
public:
69+
/* In the case of a restricted path (legacy mode), the result consists of a single value */
7170
using JsonV1Result = std::optional<T>;
71+
72+
/* In the case of an enhanced path (starts with $), the result is an array of multiple values */
7273
using JsonV2Result = std::vector<T>;
7374

7475
JsonCallbackResult() = default;
@@ -82,7 +83,7 @@ template <typename T> class JsonCallbackResult {
8283

8384
void AddValue(T&& value) {
8485
if (IsV1()) {
85-
details::OptionalEmplace(AsV1(), std::forward<T>(value));
86+
details::OptionalEmplace(std::forward<T>(value), &AsV1());
8687
} else {
8788
AsV2().emplace_back(std::forward<T>(value));
8889
}
@@ -100,6 +101,14 @@ template <typename T> class JsonCallbackResult {
100101
return std::get<JsonV2Result>(result_);
101102
}
102103

104+
const JsonV1Result& AsV1() const {
105+
return std::get<JsonV1Result>(result_);
106+
}
107+
108+
const JsonV2Result& AsV2() const {
109+
return std::get<JsonV2Result>(result_);
110+
}
111+
103112
public:
104113
std::error_code error_code;
105114

src/server/json_family.cc

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
#include "core/json/json_object.h"
2121
#include "core/json/path.h"
2222
#include "facade/cmd_arg_parser.h"
23-
#include "facade/error.h"
2423
#include "facade/op_status.h"
2524
#include "server/acl/acl_commands_def.h"
2625
#include "server/command_registry.h"
@@ -109,49 +108,53 @@ ParseResult<WrappedJsonPath> ParseJsonPath(std::string_view path) {
109108

110109
namespace reply_generic {
111110

112-
template <typename T> void Send(RedisReplyBuilder& rb, T value) = delete;
111+
template <typename T> void Send(T value, RedisReplyBuilder* rb) = delete;
113112

114-
template <> void Send(RedisReplyBuilder& rb, std::size_t value) {
115-
rb.SendLong(value);
113+
template <> void Send(std::size_t value, RedisReplyBuilder* rb) {
114+
rb->SendLong(value);
116115
}
117116

118-
template <typename T> void Send(RedisReplyBuilder& rb, const std::optional<T>& opt) {
117+
template <typename T> void Send(const std::optional<T>& opt, RedisReplyBuilder* rb) {
119118
if (opt.has_value()) {
120-
Send(rb, opt.value());
119+
Send(opt.value(), rb);
121120
} else {
122-
rb.SendNull();
121+
rb->SendNull();
123122
}
124123
}
125124

126-
template <typename T> void Send(RedisReplyBuilder& rb, const std::vector<T>& vec) {
125+
template <typename T> void Send(const std::vector<T>& vec, RedisReplyBuilder* rb) {
127126
if (vec.empty()) {
128-
rb.SendNullArray();
127+
rb->SendNullArray();
129128
} else {
130-
rb.StartArray(vec.size());
129+
rb->StartArray(vec.size());
131130
for (auto&& x : vec) {
132-
Send(rb, x);
131+
Send(x, rb);
133132
}
134133
}
135134
}
136135

137-
template <typename T> void Send(RedisReplyBuilder& rb, JsonCallbackResult<T>& result) {
136+
template <typename T> void Send(const JsonCallbackResult<T>& result, RedisReplyBuilder* rb) {
138137
if (result.error_code) {
139-
rb.SendError(result.error_code.message(), kSyntaxErrType); // todo edit
138+
rb->SendError(result.error_code.message());
140139
return;
141140
}
142141

143142
if (result.IsV1()) {
144-
Send(rb, result.AsV1());
143+
/* The specified path was restricted (JSON legacy mode), then the result consists only of a
144+
* single value */
145+
Send(result.AsV1(), rb);
145146
} else {
146-
Send(rb, result.AsV2());
147+
/* The specified path was enhanced (starts with '$'), then the result is an array of multiple
148+
* values */
149+
Send(result.AsV2(), rb);
147150
}
148151
}
149152

150-
template <typename T> void Send(RedisReplyBuilder& rb, OpResult<T>& result) {
153+
template <typename T> void Send(const OpResult<T>& result, RedisReplyBuilder* rb) {
151154
if (result) {
152-
Send(rb, result.value());
155+
Send(result.value(), rb);
153156
} else {
154-
rb.SendError(result.status());
157+
rb->SendError(result.status());
155158
}
156159
}
157160

@@ -1965,7 +1968,7 @@ void JsonFamily::StrAppend(CmdArgList args, ConnectionContext* cntx) {
19651968
Transaction* trans = cntx->transaction;
19661969
auto result = trans->ScheduleSingleHopT(std::move(cb));
19671970
auto* rb = static_cast<RedisReplyBuilder*>(cntx->reply_builder());
1968-
reply_generic::Send(*rb, result);
1971+
reply_generic::Send(result, rb);
19691972
}
19701973

19711974
void JsonFamily::ObjKeys(CmdArgList args, ConnectionContext* cntx) {

0 commit comments

Comments
 (0)