Skip to content

Commit f8e47b3

Browse files
coryancoryan
committed
feat(common): support response in long-running operations
Some long-running operations return their value in the `response` field, while others use the `metadata` field, with this change both are supported.
1 parent 0dfde77 commit f8e47b3

5 files changed

Lines changed: 197 additions & 25 deletions

‎google/cloud/internal/async_long_running_operation.h‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ namespace cloud {
3636
inline namespace GOOGLE_CLOUD_CPP_NS {
3737
namespace internal {
3838

39+
template <typename ReturnType>
40+
using LongRunningOperationValueExtractor = std::function<StatusOr<ReturnType>(
41+
StatusOr<google::longrunning::Operation>, std::string const&)>;
42+
3943
/**
4044
* Asynchronously starts and polls a long-running operation.
4145
*
@@ -108,37 +112,40 @@ template <typename ReturnType, typename RequestType, typename StartFunctor,
108112
future<StatusOr<ReturnType>> AsyncLongRunningOperation(
109113
google::cloud::CompletionQueue cq, RequestType&& request,
110114
StartFunctor&& start, AsyncPollLongRunningOperation poll,
115+
LongRunningOperationValueExtractor<ReturnType> value_extractor,
111116
std::unique_ptr<RetryPolicyType> retry_policy,
112117
std::unique_ptr<BackoffPolicy> backoff_policy, Idempotency idempotent,
113118
std::unique_ptr<PollingPolicy> polling_policy, char const* location) {
119+
using ::google::longrunning::Operation;
114120
auto operation =
115121
AsyncRetryLoop(std::move(retry_policy), std::move(backoff_policy),
116122
idempotent, cq, std::forward<StartFunctor>(start),
117123
std::forward<RequestType>(request), location);
118124
struct MoveCapture {
119125
google::cloud::CompletionQueue cq;
120126
AsyncPollLongRunningOperation poll;
127+
LongRunningOperationValueExtractor<ReturnType> value_extractor;
121128
std::unique_ptr<PollingPolicy> polling_policy;
122129
std::string location;
123130

124-
future<StatusOr<ReturnType>> operator()(
125-
future<StatusOr<google::longrunning::Operation>> f) {
131+
future<StatusOr<ReturnType>> operator()(future<StatusOr<Operation>> f) {
126132
auto op = f.get();
127133
if (!op) {
128134
return make_ready_future(StatusOr<ReturnType>(std::move(op).status()));
129135
}
130-
auto loc = this->location;
136+
auto loc = std::move(location);
137+
auto extractor = std::move(value_extractor);
131138
return AsyncPollingLoop(std::move(cq), *std::move(op), std::move(poll),
132139
std::move(polling_policy), std::move(location))
133-
.then([loc](future<StatusOr<google::longrunning::Operation>> g) {
134-
return ExtractLongRunningResult<ReturnType>(g.get(), loc);
140+
.then([extractor, loc](future<StatusOr<Operation>> g) {
141+
return extractor(g.get(), loc);
135142
});
136143
}
137144
};
138145

139-
return operation.then(MoveCapture{std::move(cq), std::move(poll),
140-
std::move(polling_policy),
141-
std::string{location}});
146+
return operation.then(
147+
MoveCapture{std::move(cq), std::move(poll), std::move(value_extractor),
148+
std::move(polling_policy), std::string{location}});
142149
}
143150

144151
} // namespace internal

‎google/cloud/internal/async_long_running_operation_test.cc‎

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ std::unique_ptr<BackoffPolicy> TestBackoffPolicy() {
6868
return ExponentialBackoffPolicy(us(100), us(100), 2.0).clone();
6969
}
7070

71-
TEST(AsyncLongRunningTest, RequestPollThenSuccess) {
71+
TEST(AsyncLongRunningTest, RequestPollThenSuccessMetadata) {
7272
Instance expected;
7373
expected.set_name("test-instance-name");
7474
google::longrunning::Operation starting_op;
@@ -117,8 +117,66 @@ TEST(AsyncLongRunningTest, RequestPollThenSuccess) {
117117
google::longrunning::GetOperationRequest const& request) {
118118
return mock->AsyncGetOperation(cq, std::move(context), request);
119119
},
120-
TestRetryPolicy(), TestBackoffPolicy(), Idempotency::kIdempotent,
121-
std::move(policy), "test-function")
120+
&ExtractLongRunningResultMetadata<Instance>, TestRetryPolicy(),
121+
TestBackoffPolicy(), Idempotency::kIdempotent, std::move(policy),
122+
"test-function")
123+
.get();
124+
ASSERT_THAT(actual, IsOk());
125+
EXPECT_THAT(*actual, IsProtoEqual(expected));
126+
}
127+
128+
TEST(AsyncLongRunningTest, RequestPollThenSuccessResponse) {
129+
Instance expected;
130+
expected.set_name("test-instance-name");
131+
google::longrunning::Operation starting_op;
132+
starting_op.set_name("test-op-name");
133+
google::longrunning::Operation done_op = starting_op;
134+
done_op.set_done(true);
135+
done_op.mutable_response()->PackFrom(expected);
136+
137+
auto mock_cq = std::make_shared<MockCompletionQueueImpl>();
138+
EXPECT_CALL(*mock_cq, MakeRelativeTimer)
139+
.WillOnce([](std::chrono::nanoseconds) {
140+
return make_ready_future(
141+
make_status_or(std::chrono::system_clock::now()));
142+
});
143+
CompletionQueue cq(mock_cq);
144+
145+
auto mock = std::make_shared<MockStub>();
146+
EXPECT_CALL(*mock, AsyncCreateInstance)
147+
.WillOnce([&](CompletionQueue&, std::unique_ptr<grpc::ClientContext>,
148+
CreateInstanceRequest const&) {
149+
return make_ready_future(make_status_or(starting_op));
150+
});
151+
EXPECT_CALL(*mock, AsyncGetOperation)
152+
.WillOnce([&](CompletionQueue&, std::unique_ptr<grpc::ClientContext>,
153+
google::longrunning::GetOperationRequest const&) {
154+
return make_ready_future(make_status_or(done_op));
155+
});
156+
auto policy = absl::make_unique<MockPollingPolicy>();
157+
EXPECT_CALL(*policy, clone()).Times(0);
158+
EXPECT_CALL(*policy, OnFailure).Times(0);
159+
EXPECT_CALL(*policy, WaitPeriod)
160+
.WillRepeatedly(Return(std::chrono::milliseconds(1)));
161+
CreateInstanceRequest request;
162+
request.set_parent("test-parent");
163+
request.set_instance_id("test-instance-id");
164+
auto actual =
165+
AsyncLongRunningOperation<Instance>(
166+
cq, std::move(request),
167+
[mock](CompletionQueue& cq,
168+
std::unique_ptr<grpc::ClientContext> context,
169+
CreateInstanceRequest const& request) {
170+
return mock->AsyncCreateInstance(cq, std::move(context), request);
171+
},
172+
[mock](CompletionQueue& cq,
173+
std::unique_ptr<grpc::ClientContext> context,
174+
google::longrunning::GetOperationRequest const& request) {
175+
return mock->AsyncGetOperation(cq, std::move(context), request);
176+
},
177+
&ExtractLongRunningResultResponse<Instance>, TestRetryPolicy(),
178+
TestBackoffPolicy(), Idempotency::kIdempotent, std::move(policy),
179+
"test-function")
122180
.get();
123181
ASSERT_THAT(actual, IsOk());
124182
EXPECT_THAT(*actual, IsProtoEqual(expected));

‎google/cloud/internal/extract_long_running_result.cc‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ namespace cloud {
2020
inline namespace GOOGLE_CLOUD_CPP_NS {
2121
namespace internal {
2222

23-
Status ExtractOperationResultImpl(
23+
Status ExtractOperationResultMetadataImpl(
2424
StatusOr<google::longrunning::Operation> op,
2525
google::protobuf::Message& result,
2626
absl::FunctionRef<bool(google::protobuf::Any const&)> validate_any,
@@ -46,6 +46,32 @@ Status ExtractOperationResultImpl(
4646
return Status{};
4747
}
4848

49+
Status ExtractOperationResultResponseImpl(
50+
StatusOr<google::longrunning::Operation> op,
51+
google::protobuf::Message& result,
52+
absl::FunctionRef<bool(google::protobuf::Any const&)> validate_any,
53+
std::string const& location) {
54+
if (!op) return std::move(op).status();
55+
if (op->has_error()) return MakeStatusFromRpcError(op->error());
56+
if (!op->has_response()) {
57+
return Status(StatusCode::kInternal,
58+
location +
59+
"() cannot extract value from operation without error or "
60+
"response, name=" +
61+
op->name());
62+
}
63+
google::protobuf::Any const& any = op->response();
64+
if (!validate_any(any)) {
65+
return Status(
66+
StatusCode::kInternal,
67+
location +
68+
"() operation completed with an invalid response type, name=" +
69+
op->name());
70+
}
71+
any.UnpackTo(&result);
72+
return Status{};
73+
}
74+
4975
} // namespace internal
5076
} // namespace GOOGLE_CLOUD_CPP_NS
5177
} // namespace cloud

‎google/cloud/internal/extract_long_running_result.h‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,23 +28,48 @@ inline namespace GOOGLE_CLOUD_CPP_NS {
2828
namespace internal {
2929

3030
/// Extracts the value (or error) from a completed long-running operation
31-
Status ExtractOperationResultImpl(
31+
Status ExtractOperationResultMetadataImpl(
3232
StatusOr<google::longrunning::Operation> op,
3333
google::protobuf::Message& result,
3434
absl::FunctionRef<bool(google::protobuf::Any const&)> validate_any,
3535
std::string const& location);
3636

37+
/// Extracts the value (or error) from a completed long-running operation
38+
Status ExtractOperationResultResponseImpl(
39+
StatusOr<google::longrunning::Operation> op,
40+
google::protobuf::Message& result,
41+
absl::FunctionRef<bool(google::protobuf::Any const&)> validate_any,
42+
std::string const& location);
43+
44+
/**
45+
* Extracts the value from a completed long-running operation.
46+
*
47+
* This helper is used in `AsyncLongRunningOperation()` to extract the value (or
48+
* error) from a completed long-running operation.
49+
*/
50+
template <typename ReturnType>
51+
StatusOr<ReturnType> ExtractLongRunningResultMetadata(
52+
StatusOr<google::longrunning::Operation> op, std::string const& location) {
53+
ReturnType result;
54+
auto status = ExtractOperationResultMetadataImpl(
55+
std::move(op), result,
56+
[](google::protobuf::Any const& any) { return any.Is<ReturnType>(); },
57+
location);
58+
if (!status.ok()) return status;
59+
return result;
60+
}
61+
3762
/**
3863
* Extracts the value from a completed long-running operation.
3964
*
4065
* This helper is used in `AsyncLongRunningOperation()` to extract the value (or
4166
* error) from a completed long-running operation.
4267
*/
4368
template <typename ReturnType>
44-
StatusOr<ReturnType> ExtractLongRunningResult(
69+
StatusOr<ReturnType> ExtractLongRunningResultResponse(
4570
StatusOr<google::longrunning::Operation> op, std::string const& location) {
4671
ReturnType result;
47-
auto status = ExtractOperationResultImpl(
72+
auto status = ExtractOperationResultResponseImpl(
4873
std::move(op), result,
4974
[](google::protobuf::Any const& any) { return any.Is<ReturnType>(); },
5075
location);

‎google/cloud/internal/extract_long_running_result_test.cc‎

Lines changed: 66 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,46 +32,102 @@ using ::google::cloud::testing_util::StatusIs;
3232
using ::google::longrunning::Operation;
3333
using ::testing::HasSubstr;
3434

35-
TEST(ExtractLongRunningResultTest, DoneWithSuccess) {
35+
TEST(ExtractLongRunningResultTest, MetadataDoneWithSuccess) {
3636
Instance expected;
3737
expected.set_name("test-instance-admin");
3838
google::longrunning::Operation op;
3939
op.set_done(true);
4040
op.mutable_metadata()->PackFrom(expected);
41-
auto const actual = ExtractLongRunningResult<Instance>(op, "test-function");
41+
auto const actual =
42+
ExtractLongRunningResultMetadata<Instance>(op, "test-function");
4243
ASSERT_STATUS_OK(actual);
4344
EXPECT_THAT(*actual, IsProtoEqual(expected));
4445
}
4546

46-
TEST(ExtractLongRunningResultTest, DoneWithError) {
47+
TEST(ExtractLongRunningResultTest, MetadataDoneWithError) {
4748
google::longrunning::Operation op;
4849
op.set_done(true);
4950
op.mutable_error()->set_code(grpc::StatusCode::PERMISSION_DENIED);
5051
op.mutable_error()->set_message("uh-oh");
51-
auto const actual = ExtractLongRunningResult<Instance>(op, "test-function");
52+
auto const actual =
53+
ExtractLongRunningResultMetadata<Instance>(op, "test-function");
5254
EXPECT_THAT(actual,
5355
StatusIs(StatusCode::kPermissionDenied, HasSubstr("uh-oh")));
5456
}
5557

56-
TEST(ExtractLongRunningResultTest, DoneWithoutResult) {
58+
TEST(ExtractLongRunningResultTest, MetadataDoneWithoutResult) {
5759
google::longrunning::Operation op;
5860
op.set_done(true);
59-
auto const actual = ExtractLongRunningResult<Instance>(op, "test-function");
61+
auto const actual =
62+
ExtractLongRunningResultMetadata<Instance>(op, "test-function");
6063
EXPECT_THAT(actual, StatusIs(StatusCode::kInternal));
6164
}
6265

63-
TEST(ExtractLongRunningResultTest, DoneWithInvalidContent) {
66+
TEST(ExtractLongRunningResultTest, MetadataDoneWithInvalidContent) {
6467
google::longrunning::Operation op;
6568
op.set_done(true);
6669
op.mutable_metadata()->PackFrom(google::protobuf::Empty{});
67-
auto const actual = ExtractLongRunningResult<Instance>(op, "test-function");
70+
auto const actual =
71+
ExtractLongRunningResultMetadata<Instance>(op, "test-function");
72+
EXPECT_THAT(actual, StatusIs(StatusCode::kInternal,
73+
AllOf(HasSubstr("test-function"),
74+
HasSubstr("invalid metadata type"))));
75+
}
76+
77+
TEST(ExtractLongRunningResultTest, MetadataError) {
78+
auto const expected = Status{StatusCode::kPermissionDenied, "uh-oh"};
79+
auto const actual =
80+
ExtractLongRunningResultMetadata<Instance>(expected, "test-function");
81+
ASSERT_THAT(actual, Not(IsOk()));
82+
EXPECT_EQ(expected, actual.status());
83+
}
84+
85+
TEST(ExtractLongRunningResultTest, ResponseDoneWithSuccess) {
86+
Instance expected;
87+
expected.set_name("test-instance-admin");
88+
google::longrunning::Operation op;
89+
op.set_done(true);
90+
op.mutable_response()->PackFrom(expected);
91+
auto const actual =
92+
ExtractLongRunningResultResponse<Instance>(op, "test-function");
93+
ASSERT_STATUS_OK(actual);
94+
EXPECT_THAT(*actual, IsProtoEqual(expected));
95+
}
96+
97+
TEST(ExtractLongRunningResultTest, ResponseDoneWithError) {
98+
google::longrunning::Operation op;
99+
op.set_done(true);
100+
op.mutable_error()->set_code(grpc::StatusCode::PERMISSION_DENIED);
101+
op.mutable_error()->set_message("uh-oh");
102+
auto const actual =
103+
ExtractLongRunningResultResponse<Instance>(op, "test-function");
104+
EXPECT_THAT(actual,
105+
StatusIs(StatusCode::kPermissionDenied, HasSubstr("uh-oh")));
106+
}
107+
108+
TEST(ExtractLongRunningResultTest, ResponseDoneWithoutResult) {
109+
google::longrunning::Operation op;
110+
op.set_done(true);
111+
auto const actual =
112+
ExtractLongRunningResultResponse<Instance>(op, "test-function");
68113
EXPECT_THAT(actual, StatusIs(StatusCode::kInternal));
69114
}
70115

71-
TEST(ExtractLongRunningResultTest, Error) {
116+
TEST(ExtractLongRunningResultTest, ResponseDoneWithInvalidContent) {
117+
google::longrunning::Operation op;
118+
op.set_done(true);
119+
op.mutable_response()->PackFrom(google::protobuf::Empty{});
120+
auto const actual =
121+
ExtractLongRunningResultResponse<Instance>(op, "test-function");
122+
EXPECT_THAT(actual, StatusIs(StatusCode::kInternal,
123+
AllOf(HasSubstr("test-function"),
124+
HasSubstr("invalid response type"))));
125+
}
126+
127+
TEST(ExtractLongRunningResultTest, ResponseError) {
72128
auto const expected = Status{StatusCode::kPermissionDenied, "uh-oh"};
73129
auto const actual =
74-
ExtractLongRunningResult<Instance>(expected, "test-function");
130+
ExtractLongRunningResultResponse<Instance>(expected, "test-function");
75131
ASSERT_THAT(actual, Not(IsOk()));
76132
EXPECT_EQ(expected, actual.status());
77133
}

0 commit comments

Comments
 (0)