Skip to content

Commit ff31dc8

Browse files
scotthartscotthart
authored
refactor(rest): ProtoRequestToJsonPayload signature (#12741)
1 parent 82eb0e5 commit ff31dc8

File tree

3 files changed

+25
-31
lines changed

3 files changed

+25
-31
lines changed

‎google/cloud/internal/rest_stub_helpers.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@ Status RestResponseToProto(google::protobuf::Message& destination,
4545
return {};
4646
}
4747

48-
Status ProtoRequestToJsonPayload(google::protobuf::Message const& request,
49-
bool preserve_proto_field_names,
50-
std::string& json_payload) {
48+
StatusOr<std::string> ProtoRequestToJsonPayload(
49+
google::protobuf::Message const& request, bool preserve_proto_field_names) {
50+
std::string json_payload;
5151
google::protobuf::util::JsonPrintOptions print_options;
5252
print_options.preserve_proto_field_names = preserve_proto_field_names;
5353
auto proto_to_json_status = google::protobuf::util::MessageToJsonString(
@@ -60,7 +60,7 @@ Status ProtoRequestToJsonPayload(google::protobuf::Message const& request,
6060
.WithReason("Failure converting proto request to HTTP")
6161
.WithMetadata("message_type", request.GetTypeName()));
6262
}
63-
return {};
63+
return json_payload;
6464
}
6565

6666
rest_internal::RestRequest CreateRestRequest(

‎google/cloud/internal/rest_stub_helpers.h‎

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,8 @@ std::vector<std::pair<std::string, std::string>> TrimEmptyQueryParameters(
3636
Status RestResponseToProto(google::protobuf::Message& destination,
3737
RestResponse&& rest_response);
3838

39-
Status ProtoRequestToJsonPayload(google::protobuf::Message const& request,
40-
bool preserve_proto_field_names,
41-
std::string& json_payload);
39+
StatusOr<std::string> ProtoRequestToJsonPayload(
40+
google::protobuf::Message const& request, bool preserve_proto_field_names);
4241

4342
rest_internal::RestRequest CreateRestRequest(
4443
std::string path,
@@ -93,15 +92,14 @@ StatusOr<Response> Patch(
9392
rest_internal::RestClient& client, rest_internal::RestContext& rest_context,
9493
Request const& request, bool preserve_proto_field_names, std::string path,
9594
std::vector<std::pair<std::string, std::string>> query_params = {}) {
96-
std::string json_payload;
97-
auto status = ProtoRequestToJsonPayload(request, preserve_proto_field_names,
98-
json_payload);
99-
if (!status.ok()) return status;
95+
auto json_payload =
96+
ProtoRequestToJsonPayload(request, preserve_proto_field_names);
97+
if (!json_payload.ok()) return std::move(json_payload).status();
10098
auto rest_request =
10199
CreateRestRequest(std::move(path), std::move(query_params));
102100
rest_request.AddHeader("content-type", "application/json");
103101
auto response = client.Patch(rest_context, rest_request,
104-
{absl::MakeConstSpan(json_payload)});
102+
{absl::MakeConstSpan(*json_payload)});
105103
if (!response.ok()) return response.status();
106104
return RestResponseToProto<Response>(std::move(**response));
107105
}
@@ -111,15 +109,14 @@ StatusOr<Response> Post(
111109
rest_internal::RestClient& client, rest_internal::RestContext& rest_context,
112110
Request const& request, bool preserve_proto_field_names, std::string path,
113111
std::vector<std::pair<std::string, std::string>> query_params = {}) {
114-
std::string json_payload;
115-
auto status = ProtoRequestToJsonPayload(request, preserve_proto_field_names,
116-
json_payload);
117-
if (!status.ok()) return status;
112+
auto json_payload =
113+
ProtoRequestToJsonPayload(request, preserve_proto_field_names);
114+
if (!json_payload.ok()) return std::move(json_payload).status();
118115
auto rest_request =
119116
CreateRestRequest(std::move(path), std::move(query_params));
120117
rest_request.AddHeader("content-type", "application/json");
121118
auto response = client.Post(rest_context, rest_request,
122-
{absl::MakeConstSpan(json_payload)});
119+
{absl::MakeConstSpan(*json_payload)});
123120
if (!response.ok()) return response.status();
124121
return RestResponseToProto<Response>(std::move(**response));
125122
}
@@ -129,15 +126,14 @@ Status Post(
129126
rest_internal::RestClient& client, rest_internal::RestContext& rest_context,
130127
Request const& request, bool preserve_proto_field_names, std::string path,
131128
std::vector<std::pair<std::string, std::string>> query_params = {}) {
132-
std::string json_payload;
133-
auto status = ProtoRequestToJsonPayload(request, preserve_proto_field_names,
134-
json_payload);
135-
if (!status.ok()) return status;
129+
auto json_payload =
130+
ProtoRequestToJsonPayload(request, preserve_proto_field_names);
131+
if (!json_payload.ok()) return std::move(json_payload).status();
136132
auto rest_request =
137133
CreateRestRequest(std::move(path), std::move(query_params));
138134
rest_request.AddHeader("content-type", "application/json");
139135
auto response = client.Post(rest_context, rest_request,
140-
{absl::MakeConstSpan(json_payload)});
136+
{absl::MakeConstSpan(*json_payload)});
141137
if (!response.ok()) return response.status();
142138
return AsStatus(std::move(**response));
143139
}
@@ -147,15 +143,14 @@ StatusOr<Response> Put(
147143
rest_internal::RestClient& client, rest_internal::RestContext& rest_context,
148144
Request const& request, bool preserve_proto_field_names, std::string path,
149145
std::vector<std::pair<std::string, std::string>> query_params = {}) {
150-
std::string json_payload;
151-
auto status = ProtoRequestToJsonPayload(request, preserve_proto_field_names,
152-
json_payload);
153-
if (!status.ok()) return status;
146+
auto json_payload =
147+
ProtoRequestToJsonPayload(request, preserve_proto_field_names);
148+
if (!json_payload.ok()) return std::move(json_payload).status();
154149
auto rest_request =
155150
CreateRestRequest(std::move(path), std::move(query_params));
156151
rest_request.AddHeader("content-type", "application/json");
157152
auto response = client.Put(rest_context, rest_request,
158-
{absl::MakeConstSpan(json_payload)});
153+
{absl::MakeConstSpan(*json_payload)});
159154
if (!response.ok()) return response.status();
160155
return RestResponseToProto<Response>(std::move(**response));
161156
}

‎google/cloud/internal/rest_stub_helpers_test.cc‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
3232
namespace {
3333

3434
using ::google::cloud::testing_util::IsOk;
35+
using ::google::cloud::testing_util::IsOkAndHolds;
3536
using ::google::cloud::testing_util::MakeMockHttpPayloadSuccess;
3637
using ::google::cloud::testing_util::MockRestClient;
3738
using ::google::cloud::testing_util::MockRestResponse;
@@ -266,14 +267,12 @@ auto constexpr kJsonUpdateResponse = R"(
266267
)";
267268

268269
TEST(RestStubHelpers, ProtoRequestToJsonPayloadSuccess) {
269-
std::string json_payload;
270270
google::longrunning::OperationInfo proto_request;
271271
proto_request.set_response_type("response_value");
272272
proto_request.set_metadata_type("metadata_value");
273273

274-
auto status = ProtoRequestToJsonPayload(proto_request, true, json_payload);
275-
ASSERT_THAT(status, IsOk());
276-
EXPECT_THAT(json_payload, Eq(kJsonUpdatePayload));
274+
auto json_payload = ProtoRequestToJsonPayload(proto_request, true);
275+
EXPECT_THAT(json_payload, IsOkAndHolds(Eq(kJsonUpdatePayload)));
277276
}
278277

279278
TEST(RestStubHelpers, Patch) {

0 commit comments

Comments
 (0)