Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
coryan committed Oct 13, 2022
1 parent eaea194 commit 5942541
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 10 deletions.
10 changes: 4 additions & 6 deletions google/cloud/pubsub/subscriber.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,17 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
Subscriber::Subscriber(std::shared_ptr<SubscriberConnection> connection,
Options opts)
: connection_(std::move(connection)),
options_(google::cloud::internal::MergeOptions(std::move(opts),
connection_->options())) {}
options_(
internal::MergeOptions(std::move(opts), connection_->options())) {}

future<Status> Subscriber::Subscribe(ApplicationCallback cb, Options opts) {
google::cloud::internal::OptionsSpan span(
google::cloud::internal::MergeOptions(std::move(opts), options_));
internal::OptionsSpan span(internal::MergeOptions(std::move(opts), options_));
return connection_->Subscribe({std::move(cb)});
}

future<Status> Subscriber::Subscribe(ExactlyOnceApplicationCallback cb,
Options opts) {
google::cloud::internal::OptionsSpan span(
google::cloud::internal::MergeOptions(std::move(opts), options_));
internal::OptionsSpan span(internal::MergeOptions(std::move(opts), options_));
return connection_->ExactlyOnceSubscribe({std::move(cb)});
}

Expand Down
12 changes: 8 additions & 4 deletions google/cloud/pubsub/subscriber_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -164,14 +164,14 @@ TEST(SubscriberTest, OptionsFunctionOverrides) {
.set<TestOptionC>("test-c")));
EXPECT_CALL(*mock, Subscribe).WillOnce([](auto const&) {
auto const& current = google::cloud::internal::CurrentOptions();
EXPECT_EQ(current.get<TestOptionA>(), "override-a");
EXPECT_EQ(current.get<TestOptionA>(), "override-a1");
EXPECT_EQ(current.get<TestOptionB>(), "override-b1");
EXPECT_EQ(current.get<TestOptionC>(), "test-c");
return make_ready_future(Status{});
});
EXPECT_CALL(*mock, ExactlyOnceSubscribe).WillOnce([](auto const&) {
auto const& current = google::cloud::internal::CurrentOptions();
EXPECT_EQ(current.get<TestOptionA>(), "override-a");
EXPECT_EQ(current.get<TestOptionA>(), "override-a2");
EXPECT_EQ(current.get<TestOptionB>(), "override-b2");
EXPECT_EQ(current.get<TestOptionC>(), "test-c");
return make_ready_future(Status{});
Expand All @@ -180,12 +180,16 @@ TEST(SubscriberTest, OptionsFunctionOverrides) {
Subscriber subscriber(mock, Options{}.set<TestOptionA>("override-a"));
ASSERT_STATUS_OK(subscriber
.Subscribe([](Message const&, AckHandler const&) {},
Options{}.set<TestOptionB>("override-b1"))
Options{}
.set<TestOptionA>("override-a1")
.set<TestOptionB>("override-b1"))
.get());
ASSERT_STATUS_OK(
subscriber
.Subscribe([](Message const&, ExactlyOnceAckHandler const&) {},
Options{}.set<TestOptionB>("override-b2"))
Options{}
.set<TestOptionA>("override-a2")
.set<TestOptionB>("override-b2"))
.get());
}

Expand Down

0 comments on commit 5942541

Please sign in to comment.